Try to obtain host names from another address of the same device when nothing else is available #879

DL6ER · 2020-08-31T17:03:42Z

By submitting this pull request, I confirm the following:

I have read and understood the contributors guide.
I have checked that another pull request for this purpose does not exist.
I have considered, and confirmed that this submission will be valuable to others.
I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10

Try to obtain host names from another address of the same device when there is none for the exact address (may happen, e.g., for IPv6 addresses). This is currently broken in development due to names having moved into the network_addresses table.

This fixes a bug mentioned on Discourse (see link below).

… there is none for the exact address (may happen, e.g., for IPv6 addresses) Signed-off-by: DL6ER <dl6er@dl6er.de>

pralor-bot · 2020-08-31T17:03:52Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/feedback-for-allow-defining-clients-by-their-mac-address-host-name-and-networking-interface/32324/219

Signed-off-by: DL6ER <dl6er@dl6er.de>

ampfinger · 2020-08-31T18:41:49Z

And that was the trick!
Now it is working, ipv6 without name in network_addresses now shows the name from the ipv4 in dashboard and query log :)

Thanks a lot!

DL6ER · 2020-08-31T19:15:42Z

src/database/network-table.c

-char *__attribute__((malloc)) getDatabaseHostname(const char *ipaddr)
+// Get hardware address of device identified by IP address
+char *__attribute__((malloc)) getMACfromIP(const char *ipaddr)


This is not correct, git screwed up the diff. I removed the getDatabaseHostname() function altogether as (almost) the same is done by getNameFromIP() below.

DL6ER · 2020-08-31T19:16:39Z

Confirmed working by @ampfinger

The change is much less than shown by GitHub. It's diff-generator messed up the actual change and thought I'd rewritten a lot more.

dschaper · 2020-08-31T19:58:19Z

Can you do me a favor and force push your latest changes? I'm not all that comfortable merging in things if the diff is displaying incorrect information.

DL6ER · 2020-08-31T20:12:13Z

I don't think this is possible, it is the entire change that is confusing GitHub. The individual commits are fine:

It's the entirety of these commits that is causing headaches for GitHub.
Even a direct comparison is fine:
https://github.com/pi-hole/FTL/compare/a598022a9f9a9f944db5724302c37f9e52bec811..9bdfee2e1937a17e8d4d212dbceef3f4ae083cc4

Just the PR diff is broken.

dschaper · 2020-08-31T20:41:58Z

So https://github.com/pi-hole/FTL/compare/development..9bdfee2e1937a17e8d4d212dbceef3f4ae083cc4 looks okay?

DL6ER · 2020-08-31T21:09:32Z

Yes, the changes are fine.

ampfinger · 2020-09-10T15:30:18Z

I have updated pihole -up some days ago (maybe two) and so pi-hole core was updated.
Dont know if it is related but now network table is not updating anymore. Flush network table and restart pihole were not working.
In Pihole-FTL.log there is an error
"Trying to free NULL pointer in ResolveAndAddHostname".

Debug token is
https://tricorder.pi-hole.net/4ko5mxgyn4

DL6ER · 2020-09-10T19:22:59Z

network table is not updating anymore

Not at all? Are queries still being saved in the database?
Are there any other errors in the log?

The Trying to free NULL pointer is due to a missing check, however, they are actually harmless at this point and not related. I will push a fix that silences them (by not trying to free if NULL).

Signed-off-by: DL6ER <dl6er@dl6er.de>

ampfinger · 2020-09-10T19:55:56Z

Queries are stored correctly but the network table was 4h old (last seen field on all devices).
There were no other errors except the one above.
Maybe it was another error (without connection to this PR).
Checked it out 20min ago, errors are gone, network table is updated as before.

I apologize for the inconvenience

DL6ER · 2020-09-10T19:58:59Z

No need to apologize and no inconvenience caused. In the end we want Pi-hole to be the best it can, so this definitely was good.
My fixes were only of cosmetic nature, the changes in this PR cannot affect the database.

One last thing: Are there and results when you run

grep "getNameFromIP" /var/log/pihole-FTL.log*

?

ampfinger · 2020-09-10T20:17:38Z

First: you all do great work!
Second: Cosmetic changes are important to avoid unnecessary questions ;)
And last: empty output, this is good?

DL6ER · 2020-09-10T20:32:34Z

empty output, this is good?

Yes :-)

ampfinger · 2020-09-17T19:44:01Z

Three times this week I got this message.
Long term database and network table stopped, dns resolution works. After restart everything started working again.

 SQLite3 message: API call with invalid database connection pointer (21)
[2020-09-17 16:14:00.215 5236/T5240] SQLite3 message: misuse at line 127639 of [3bfa9cc97d] (21)
[2020-09-17 16:14:00.215 5236/T5240] ERROR: SQL query "BEGIN TRANSACTION IMMEDIATE" failed: bad parameter or other API misuse
[2020-09-17 16:14:00.215 5236/T5240] ERROR: Storing devices in network table ("BEGIN TRANSACTION IMMEDIATE") failed

Is this related to this? Otherwise I will open a new issue.

https://tricorder.pi-hole.net/in7vr9i1t5

DL6ER · 2020-09-17T20:04:18Z

@ampfinger It's unclear if this PR is the cause.

API call with invalid database connection pointer (21)

This likely means that there was an error (before this line!) with the database which is why the handle was closed prematurely. Are there any other database-related messages before this one? Your tricorder log does not show any errors, I guess you did it after restarting?

ampfinger · 2020-09-17T20:08:26Z

Yes I did it afterwards.
Error at 13:45, then I restarted. So:

Listening on Unix socket
[2020-09-17 13:46:26.993 5236/T5238] Listening on port 4711 for incoming IPv6 telnet connections
[2020-09-17 13:46:26.995 5236/T5237] Listening on port 4711 for incoming IPv4 telnet connections
[2020-09-17 13:46:26.995 5236M] Reloading DNS cache
[2020-09-17 13:46:26.996 5236M] Blocking status is enabled [2020-09-17 13:46:27.024 5236M] Compiled 0 whitelist and 0 blacklist regex filters for 43 clients in 0.8 msec
[2020-09-17 13:59:00.985 5236/T5240] Notice: Database size is 56.57 MB, deleted 713 rows
[2020-09-17 14:00:01.169 5236M] Resizing "/FTL-dns-cache" from 4096 to 8192 [2020-09-17 14:12:34.001 5236M] Resizing "/FTL-strings" from 65536 to 69632
[2020-09-17 14:13:18.864 5236M] Resizing "/FTL-dns-cache" from 8192 to 12288
[2020-09-17 14:21:09.664 5236M] Resizing "/FTL-dns-cache" from 12288 to 16384 [2020-09-17 14:21:58.793 5236M] Resizing "/FTL-strings" from 69632 to 73728
[2020-09-17 14:21:58.795 5236M] Resizing "/FTL-dns-cache" from 16384 to 20480 [2020-09-17 14:37:47.236 5236M] Resizing "/FTL-dns-cache" from 20480 to 24576
[2020-09-17 14:59:00.232 5236/T5240] Notice: Database size is 56.57 MB, deleted 665 rows
[2020-09-17 15:06:04.946 5236M] Resizing "/FTL-dns-cache" from 24576 to 28672 [2020-09-17 15:12:21.654 5236M] Resizing "/FTL-strings" from 73728 to 77824
[2020-09-17 15:46:16.677 5236M] Resizing "/FTL-dns-cache" from 28672 to 32768
[2020-09-17 16:00:00.204 5236/T5240] Notice: Database size is 56.57 MB, deleted 773 rows
[2020-09-17 16:12:48.061 5236M] Resizing "/FTL-dns-cache" from 32768 to 36864

Thats the log until error posted before

dschaper · 2020-09-17T22:28:33Z

Ping me when you confirm that this PR isn't the issue cause. Thanks!

ampfinger · 2020-09-18T05:28:41Z

Okay I found the issue, was one of my old scripts not correctly uncommented and which had not correctly unlocked db.

Sorry, my fault.

DL6ER · 2020-09-18T06:34:11Z

Thanks for finding that it wasn't me who broke the code. However, I see some need here to improve on the error messages. SQLite3 isn't always straight helpful with their texts.

@ampfinger Can you still reproduce the error locally? If so, please add

DEBUG_LOCKS=true
DEBUG_DATABASE=true

to the file /etc/pihole/pihole-FTL.conf (create if it does not exist) and run

pihole restartdns

This will result in a lot more details being shown in the log, hopefully this can tell us why the database connection pointer was invalid at that time.

ampfinger · 2020-09-18T06:38:35Z

I will set the settings and hope to reproduce it!

ampfinger · 2020-09-21T04:28:58Z

Actually this bug cant be reproduced at the moment. It didnt raise the last days.
Maybe we can close/merged this PR and I will open new issue if it comes back. At the moment I cant be sure if its related to this PR or not.
But when it is related merged in development more people can test it.

DL6ER · 2020-09-21T05:55:24Z

@dschaper you asked for a ping when this PR is ready.

pralor-bot · 2020-11-28T19:28:37Z

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-core-web-v5-2-and-ftl-v5-3-released/40909/1

Try to obtain host names from another address of the same device when…

a598022

… there is none for the exact address (may happen, e.g., for IPv6 addresses) Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER added the Bugfix label Aug 31, 2020

DL6ER added 2 commits August 31, 2020 19:50

Fix syntax error

693aa46

Signed-off-by: DL6ER <dl6er@dl6er.de>

Remove duplicate function getDatabaseHostname()

9bdfee2

Signed-off-by: DL6ER <dl6er@dl6er.de>

DL6ER commented Aug 31, 2020

View reviewed changes

DL6ER requested review from dschaper and PromoFaux August 31, 2020 19:15

DL6ER added 2 commits September 10, 2020 21:23

Do not try to free NULL pointer in resolveAndAddHostname

5c2e242

Signed-off-by: DL6ER <dl6er@dl6er.de>

Show debug messages only in debug mode

1123a21

Signed-off-by: DL6ER <dl6er@dl6er.de>

dschaper approved these changes Sep 21, 2020

View reviewed changes

DL6ER merged commit 4440bd0 into development Sep 21, 2020

DL6ER deleted the fix/dbnames branch September 21, 2020 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to obtain host names from another address of the same device when nothing else is available #879

Try to obtain host names from another address of the same device when nothing else is available #879

DL6ER commented Aug 31, 2020

pralor-bot commented Aug 31, 2020

ampfinger commented Aug 31, 2020 •

edited

Loading

DL6ER Aug 31, 2020

DL6ER commented Aug 31, 2020

dschaper commented Aug 31, 2020

DL6ER commented Aug 31, 2020

dschaper commented Aug 31, 2020

DL6ER commented Aug 31, 2020

ampfinger commented Sep 10, 2020

DL6ER commented Sep 10, 2020

ampfinger commented Sep 10, 2020

DL6ER commented Sep 10, 2020

ampfinger commented Sep 10, 2020

DL6ER commented Sep 10, 2020

ampfinger commented Sep 17, 2020 •

edited by DL6ER

Loading

DL6ER commented Sep 17, 2020

ampfinger commented Sep 17, 2020 •

edited

Loading

dschaper commented Sep 17, 2020

ampfinger commented Sep 18, 2020

DL6ER commented Sep 18, 2020

ampfinger commented Sep 18, 2020

ampfinger commented Sep 21, 2020

DL6ER commented Sep 21, 2020

pralor-bot commented Nov 28, 2020

Try to obtain host names from another address of the same device when nothing else is available #879

Try to obtain host names from another address of the same device when nothing else is available #879

Conversation

DL6ER commented Aug 31, 2020

10

pralor-bot commented Aug 31, 2020

ampfinger commented Aug 31, 2020 • edited Loading

DL6ER Aug 31, 2020

Choose a reason for hiding this comment

DL6ER commented Aug 31, 2020

dschaper commented Aug 31, 2020

DL6ER commented Aug 31, 2020

dschaper commented Aug 31, 2020

DL6ER commented Aug 31, 2020

ampfinger commented Sep 10, 2020

DL6ER commented Sep 10, 2020

ampfinger commented Sep 10, 2020

DL6ER commented Sep 10, 2020

ampfinger commented Sep 10, 2020

DL6ER commented Sep 10, 2020

ampfinger commented Sep 17, 2020 • edited by DL6ER Loading

DL6ER commented Sep 17, 2020

ampfinger commented Sep 17, 2020 • edited Loading

dschaper commented Sep 17, 2020

ampfinger commented Sep 18, 2020

DL6ER commented Sep 18, 2020

ampfinger commented Sep 18, 2020

ampfinger commented Sep 21, 2020

DL6ER commented Sep 21, 2020

pralor-bot commented Nov 28, 2020

ampfinger commented Aug 31, 2020 •

edited

Loading

ampfinger commented Sep 17, 2020 •

edited by DL6ER

Loading

ampfinger commented Sep 17, 2020 •

edited

Loading